Skip to content

Conversation

@EtienneLt
Copy link
Contributor

No description provided.

@EtienneLt EtienneLt self-assigned this Oct 22, 2025
Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: Etienne LESOT <[email protected]>
@EtienneLt EtienneLt requested a review from basseche October 23, 2025 06:31
basseche
basseche previously approved these changes Oct 23, 2025
@basseche basseche self-requested a review October 23, 2025 12:47
Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: Etienne LESOT <[email protected]>
@basseche basseche dismissed their stale review October 24, 2025 15:04

i should review again

@Mathieu-Deharbe Mathieu-Deharbe self-requested a review October 28, 2025 13:51
private void removeOlg(Branch<?> branch, OperationalLimitsGroupModificationInfos opLGModificationInfos, List<ReportNode> operationalLimitsGroupReports, OperationalLimitsGroupInfos.Applicability applicability) {
if (applicability == SIDE1 && branch.getOperationalLimitsGroup1(opLGModificationInfos.getId()).isEmpty() ||
applicability == SIDE2 && branch.getOperationalLimitsGroup2(opLGModificationInfos.getId()).isEmpty()) {
throw new PowsyblException("Cannot delete operational limit group " + opLGModificationInfos.getId() + " which has not been found in equipment on side " + applicability);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you sur we should throw here ? Why not just log an error ?

Copy link
Contributor

@Mathieu-Deharbe Mathieu-Deharbe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't review yet but there seem to be aproblem relative to the side :
Here I deleted both sides ETE2 and HIVER2 :
image
The logs display deletion only on side 1 :
image

And in the next node HIVER2 and ETE2 side 2 are still there :

image

Comment on lines +337 to +346
line.newOperationalLimitsGroup1("NewLimitsGroup2").newCurrentLimits()
.setPermanentLimit(10.0)
.add();
// side 2
line.newOperationalLimitsGroup2("NewLimitsGroup1").newCurrentLimits()
.setPermanentLimit(10.0)
.add();
line.newOperationalLimitsGroup2("NewLimitsGroup3").newCurrentLimits()
.setPermanentLimit(10.0)
.add();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason for "NewLimitsGroup2" and "NewLimitsGroup3" to have 2 different names ? Wouldn't it be more useful to have the same name in order to check that the DELETE correctly handle the applicability and doesn't delete both ?

@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 3, 2025

@Mathieu-Deharbe Mathieu-Deharbe requested review from Mathieu-Deharbe and removed request for Mathieu-Deharbe November 3, 2025 17:20
@Mathieu-Deharbe Mathieu-Deharbe requested review from Mathieu-Deharbe and removed request for Mathieu-Deharbe November 3, 2025 17:20
@Mathieu-Deharbe Mathieu-Deharbe requested review from Mathieu-Deharbe and removed request for Mathieu-Deharbe November 3, 2025 17:22
@Mathieu-Deharbe Mathieu-Deharbe self-requested a review November 3, 2025 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants